dxf: refactor error handling in task executor #57837
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @D3Hunter. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| @@ -234,22 +234,6 @@ func TestGC(t *testing.T) { | |||
| }, 10*time.Second, 500*time.Millisecond) | |||
| } | |||
|
|
|||
| func TestFrameworkSubtaskFinishedCancel(t *testing.T) { | |||
There was a problem hiding this comment.
this case test what we do when a subtask is cancelled by other routines by CancelRunningSubtask, it's actually duplicate with TestFrameworkRunSubtaskCancelOrFailed, so merge them
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #57837 +/- ##
================================================
+ Coverage 73.1430% 73.9898% +0.8468%
================================================
Files 1671 1702 +31
Lines 460633 470274 +9641
================================================
+ Hits 336921 347955 +11034
+ Misses 103006 100963 -2043
- Partials 20706 21356 +650
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| mockStepExecutor.EXPECT().Cleanup(gomock.Any()).Return(nil) | ||
| err = taskExecutor.RunStep(nil) | ||
| require.EqualError(t, err, grpcErr.Error()) | ||
| require.True(t, ctrl.Satisfied()) | ||
|
|
||
| // 9. annotate grpc cancel |
There was a problem hiding this comment.
we check context cancel by using ctx.Err now, no need to check this one
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GMHDBJD, wjhuang2016 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
What problem does this PR solve?
Issue Number: ref #57497
Problem Summary:
What changed and how does it work?
GetStepExecutoris fatal nowCleanupnow, only log it, previously we will mark a randomly subtask as failed, but if all subtasks are finished, this does nothing.onErroris removed now, I plan to add special initialized logger which will always print error stack to achieve the same effect.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.